Skip to content

Themed Submission Components#4080

Merged
tdonohue merged 10 commits intoDSpace:mainfrom
atmire:themed-SubmissionComponents_contribute-main
Apr 30, 2025
Merged

Themed Submission Components#4080
tdonohue merged 10 commits intoDSpace:mainfrom
atmire:themed-SubmissionComponents_contribute-main

Conversation

@ZahraaChreim-Atmire
Copy link

@ZahraaChreim-Atmire ZahraaChreim-Atmire commented Mar 14, 2025 •
edited by alexandrevryghem
Loading

Description

This PR makes the SubmissionFormFooterComponent, SubmissionSectionContainerComponent, and SubmissionFormComponent themeable.

Instructions for Reviewers

List of changes in this PR:

  • First, created ThemedSubmissionFormFooterComponent, ThemedSubmissionSectionContainerComponent, and ThemedSubmissionFormComponent and extended them from ThemedComponent<OriginalComponent>.
  • Second, set the themed component selectors to use the format ds-themed-${original-selector}.
  • Third, implemented getComponentName, importThemedComponent, and importUnthemedComponent.
  • Fourth, added all @Input() and @Output() properties from the original components and overrode inAndOutputNames to properly register inputs and outputs.
  • Fifth, replaced references to the original components with their themed counterparts in routing modules & standalone imports

How to test or review the PR.

  • Set the theme to custom theme in config.yml
  • Uncomment the CustomEagerThemeModule in src/themes/eager-themes.module.ts
  • Test if the new components in the custom theme folder can be themed

Checklist

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using npm run lint
  • My PR doesn't introduce circular dependencies (verified via npm run check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR aligns with Accessibility guidelines if it makes changes to the user interface.
  • My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations.
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

@tdonohue tdonohue moved this to 🙋 Needs Reviewers Assigned in DSpace 9.0 Release Mar 14, 2025
@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge label Mar 14, 2025
@FrancescoMolinaro
Copy link
Contributor

FrancescoMolinaro commented Mar 27, 2025

Hi @ZahraaChreim-Atmire , thanks for the changes! I wanted to test it locally but i noticed that the build in prod mode is failing and the tests too.
Could you please have a look into it? Thanks in advance.

@ZahraaChreim-Atmire
Copy link
Author

ZahraaChreim-Atmire commented Apr 4, 2025

Hi @ZahraaChreim-Atmire , thanks for the changes! I wanted to test it locally but i noticed that the build in prod mode is failing and the tests too. Could you please have a look into it? Thanks in advance.

Hi @FrancescoMolinaro , I've fixed the issues with the build and tests. Could you please take another look? Thanks!

FrancescoMolinaro
FrancescoMolinaro suggested changes Apr 11, 2025
View reviewed changes
Copy link
Contributor

@FrancescoMolinaro FrancescoMolinaro left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ZahraaChreim-Atmire , many thanks for the changes.
The PR looks good now, I have added just a small change request because I think one of the selector of the new themed compoents is wrong.
Could you please have a look?

Thanks in advance.

@github-project-automation github-project-automation bot moved this from 🙋 Needs Reviewers Assigned to 👀 Under Review in DSpace 9.0 Release Apr 11, 2025
@ZahraaChreim-Atmire
Copy link
Author

ZahraaChreim-Atmire commented Apr 15, 2025

Hi @ZahraaChreim-Atmire , many thanks for the changes. The PR looks good now, I have added just a small change request because I think one of the selector of the new themed compoents is wrong. Could you please have a look?

Thanks in advance.

Hi @FrancescoMolinaro, you're absolutely right, the selector was incorrect. I've updated it to ds-themed-base-submission-section-container. Let me know if anything else needs adjusting.

Thanks again!

FrancescoMolinaro
FrancescoMolinaro approved these changes Apr 29, 2025
View reviewed changes
Copy link
Contributor

@FrancescoMolinaro FrancescoMolinaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ZahraaChreim-Atmire , thanks for the update and sorry for the delay on this.
I gave the code another look and another round of test and to me this is ready to be merged.

@tdonohue tdonohue moved this from 👀 Under Review to 👍 Reviewer Approved in DSpace 9.0 Release Apr 29, 2025
tdonohue
tdonohue reviewed Apr 29, 2025
View reviewed changes
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZahraaChreim-Atmire : Thanks for this update. I tested this today & it all looks good. But, before we merge it, I have a question about one line of code below.

tdonohue
tdonohue approved these changes Apr 30, 2025
View reviewed changes
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks @ZahraaChreim-Atmire ! This looks good to me now.

@tdonohue tdonohue added this to the 9.0 milestone Apr 30, 2025
@tdonohue tdonohue merged commit 3370bda into DSpace:main Apr 30, 2025
15 checks passed
@github-project-automation github-project-automation bot moved this from 👍 Reviewer Approved to ✅ Done in DSpace 9.0 Release Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdonohue tdonohue tdonohue approved these changes

+1 more reviewer

@FrancescoMolinaro FrancescoMolinaro FrancescoMolinaro approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ZahraaChreim-Atmire ZahraaChreim-Atmire

Labels

1 APPROVAL pull request only requires a single approval to merge themes

Projects

No open projects
DSpace 9.0 Release
Status: ✅ Done

Milestone

9.0

Development

Successfully merging this pull request may close these issues.

5 participants

@ZahraaChreim-Atmire @FrancescoMolinaro @tdonohue @alexandrevryghem @Atmire-Kristof